Skip to content

l2geth: Use Google PubSub for transaction syncing#2417

Closed
Inphi wants to merge 4 commits intoethereum-optimism:developfrom
Inphi:inphi/geth-pubsub
Closed

l2geth: Use Google PubSub for transaction syncing#2417
Inphi wants to merge 4 commits intoethereum-optimism:developfrom
Inphi:inphi/geth-pubsub

Conversation

@Inphi
Copy link
Contributor

@Inphi Inphi commented Apr 5, 2022

This reverts the pub/sub feature introduced by eedbebb, with additional fixes. This change will go through nightly so it can be tested properly before being merged.

@changeset-bot
Copy link

changeset-bot bot commented Apr 5, 2022

🦋 Changeset detected

Latest commit: 8d2c620

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@eth-optimism/l2geth Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@mergify mergify bot requested review from cfromknecht and tuxcanfly April 5, 2022 02:20
@Inphi Inphi changed the title l2geth: Publish transactions to Google PubSub l2geth: Use Google PubSub for transaction syncing Apr 5, 2022
@mslipper mslipper requested a review from tynes April 5, 2022 05:10
@@ -0,0 +1,5 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can squash out this changeset

@Inphi Inphi force-pushed the inphi/geth-pubsub branch from ac01591 to 6618701 Compare April 5, 2022 22:20
@mslipper mslipper added the C-protocol-critical Category: Modifies protocol-critical code label Apr 5, 2022
@mergify mergify bot requested a review from smartcontracts April 5, 2022 22:29
@Inphi Inphi force-pushed the inphi/geth-pubsub branch from 6618701 to 6f90145 Compare April 11, 2022 19:55
@mslipper
Copy link
Collaborator

@tynes can you confirm here why we don't reset the enqueue index when the transaction fails to apply? This commit here: 6f90145

@tynes
Copy link
Contributor

tynes commented Apr 13, 2022

@tynes can you confirm here why we don't reset the enqueue index when the transaction fails to apply? This commit here: 6f90145

If a deposit causes a problem when being ingested, we don't want to halt the ingestion of all deposits by having the routine be stuck in an infinite loop. The deposit can be replayed via the contracts which will allow for it to go through. There should never be a problem when a deposit is being ingested to cause such an error

@github-actions
Copy link
Contributor

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Apr 28, 2022
@Inphi Inphi removed the Stale label Apr 28, 2022
@Inphi Inphi force-pushed the inphi/geth-pubsub branch from 6f90145 to 8d2c620 Compare May 3, 2022 17:50
@mergify
Copy link
Contributor

mergify bot commented May 4, 2022

Hey @Inphi! This PR has merge conflicts. Please fix them before continuing review.

@mergify mergify bot added the conflict label May 4, 2022
@github-actions
Copy link
Contributor

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label May 19, 2022
@Inphi Inphi removed the Stale label May 19, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2022

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Jun 3, 2022
@github-actions github-actions bot closed this Jun 8, 2022
theochap pushed a commit that referenced this pull request Dec 10, 2025
## Overview

Fixes the block label metrics. A recent PR removed them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cannon Area: cannon C-protocol-critical Category: Modifies protocol-critical code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants